Skip to content

IPv6 ACL: match protocol-number rules with meta l4proto - #14360

Open
bhouse-nexthop wants to merge 4 commits into
apache:4.22from
bhouse-nexthop:fix-ip6-acl-l4proto
Open

bhouse-nexthop wants to merge 4 commits into
apache:4.22from
bhouse-nexthop:fix-ip6-acl-l4proto

Conversation

@bhouse-nexthop

@bhouse-nexthop bhouse-nexthop commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Description

IPv6 network ACL rules given by protocol number are now matched with meta l4proto, the expression nft itself uses for tcp, udp and icmpv6 rules. Every IPv6 ACL rule then identifies the protocol the same way. A rule given by the number of an IPv6 extension header (0, 43, 44, 60, 135) is matched as that extension header (exthdr <name> exists).

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

  • New systemvm/test/TestCsAclIpv6.py checks the rendered ingress and egress rules, including extension header numbers.
  • The generated rules were loaded into nft through CsNetfilters.apply_ip6_rules() and checked against traffic.
  • meta l4proto and exthdr ... exists were checked to load on nft 1.0.6, the version in the Debian 12 systemvm template.
  • pycodestyle and pylint (as in systemvm/test/runtests.sh) report nothing new.

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.12%. Comparing base (8074c3e) to head (91794f9).

Additional details and impacted files
@@             Coverage Diff              @@
##               4.22   #14360      +/-   ##
============================================
- Coverage     18.13%   18.12%   -0.01%     
+ Complexity    16445    16443       -2     
============================================
  Files          5939     5939              
  Lines        537306   537306              
  Branches      65867    65867              
============================================
- Hits          97439    97402      -37     
- Misses       428749   428796      +47     
+ Partials      11118    11108      -10     
Flag Coverage Δ
uitests 4.04% <ø> (ø)
unittests 19.20% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bhouse-nexthop

Copy link
Copy Markdown
Collaborator Author

@vladimirpetrov @sureshanaparti could you take a look at this one? We'd like this fix to make it into the upcoming 4.22.2 release.

@Damans227

Copy link
Copy Markdown
Collaborator

thanks @bhouse-nexthop, fix looks right. good to go from my side

Match IPv6 ACL rules given by protocol number with "meta l4proto", the
expression nft itself uses for tcp, udp and icmpv6 rules, so that every
IPv6 ACL rule identifies the protocol the same way.

Signed-off-by: Brad House <bhouse@nexthop.ai>
An ACL rule given by the protocol number of an IPv6 extension header
(0, 43, 44, 60, 135) is rendered as nft's "exthdr <name> exists" for
that header, rather than as an upper-layer protocol.

Signed-off-by: Brad House <bhouse@nexthop.ai>
Signed-off-by: Brad House <bhouse@nexthop.ai>
Signed-off-by: Brad House <bhouse@nexthop.ai>
@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@bhouse-nexthop

bhouse-nexthop commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

@weizhouapache thanks for kicking the packaging. Could you also run the tests and add it to the 4.22.2 milestone? @Damans227 reviewed it on 10-08; it still needs a committer review. I understand 4.22.2 is due out late this week or early next.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19681

@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-17216)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 59509 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14360-t17216-kvm-ol8.zip
Smoke tests completed. 146 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_isolate_network_password_server Failure 45.86 test_password_server.py
test_01_primary_storage_nfs Error 0.15 test_primary_storage.py
ContextSuite context=TestStorageTags>:setup Error 0.24 test_primary_storage.py
test_01_primary_storage_scope_change Error 0.10 test_primary_storage_scope.py

@weizhouapache weizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

it may not cover all the protocol numbers, but at least it fixes the rules for some protocol numbers.

@bhouse-nexthop

Copy link
Copy Markdown
Collaborator Author

Test report: IPv6 protocol-number ACL rules on a running VPC router, with extension-header traffic

Environment

  • A CloudStack 24.0 (main) build with this PR applied, alongside some of my other open PRs.
  • Why the result carries over to 4.22:
    • Main still ships system VM template 4.22.0, and the IPv6 ACL part of configure.py is the same on 4.22 and main.
    • The VPC offering has no Firewall service, as on 4.22.
  • Three KVM hosts on Ubuntu 24.04, nested virtualisation; Ceph RBD primary storage.
  • System VM template 4.22.0 (Debian 12, nft 1.0.6). The router was created on this build, and its /opt/cloud/bin/configure.py matches the build's (md5 checked before and after every stock-script comparison).
  • A dual-stack VPC. Tier T1 holds V1 (Ubuntu 24.04), which listens on UDP 5001 and runs tcpdump on its NIC.
  • An external host on the public network sends the probe packets with scapy 2.5.0.
  • Each stage had only one rule in T1's list, so every delivered packet can be tied to that rule. Each rule is cidrlist=::/0 action=allow, ingress unless stated.
  • For the "without the PR" runs, I copied apache/main's configure.py onto the router, re-applied the list, and then restored the PR's script with restartVPC livepatch=true.

1. What the router renders

Rule protocol With the PR Stock
17 meta l4proto udp accept ip6 nexthdr udp accept
6 meta l4proto tcp accept
60 (destination options) exthdr dst exists accept ip6 nexthdr ipv6-opts accept
0 (hop-by-hop) exthdr hbh exists accept
51 (AH) meta l4proto ah accept ip6 nexthdr ah accept

Every rule loaded. The router's cloud.log has no failed nft add line over the whole run.

2. Ingress traffic (external host to V1)

Rule Packet With the PR Stock
17 IPv6/UDP delivered delivered
17 IPv6/DestOpt/UDP delivered dropped
17 IPv6/HopByHop/UDP delivered dropped
17 IPv6/AH/UDP dropped dropped
17 IPv6/TCP SYN dropped
60 IPv6/DestOpt/UDP delivered delivered
60 IPv6/UDP, IPv6/HopByHop/UDP dropped dropped (UDP)
0 IPv6/HopByHop/UDP delivered
0 IPv6/UDP, IPv6/DestOpt/UDP dropped
51 IPv6/AH/UDP, IPv6/AH (no next header) delivered delivered
51 IPv6/UDP dropped dropped
6 IPv6/TCP SYN delivered (V1 answers with a reset)
6 IPv6/UDP dropped

From V1's capture, with the PR and rule 17 only:

IP6 (next-header unknown (60) ...) <ext> > <V1>: DSTOPT (padn) 41002 > 5001: [udp sum ok] UDP, length 3
IP6 (next-header Options (0) ...) <ext> > <V1>: HBH (padn) 41006 > 5001: [udp sum ok] UDP, length 3

With the stock script and the same rule, only the plain UDP packet arrives.

  • Protocol 51 behaves the same with and without the PR. nft's meta l4proto stops at an AH header, so meta l4proto ah matches AH packets, and a UDP rule does not match UDP carried behind AH.
  • With the PR and rules 17, 60 and 51 in one list, the UDP, destination-options, hop-by-hop and AH packets are all delivered, and a TCP SYN and an ICMPv6 echo are dropped.

3. Egress traffic (V1 to the external host)

V1 sends a plain UDP datagram and one with a destination options header (IPV6_DSTOPTS socket option). The capture runs on the external host.

Egress rule Plain UDP DestOpt/UDP
17, with the PR (ip6 daddr ::/0 meta l4proto udp accept) delivered delivered
17, stock (ip6 daddr ::/0 ip6 nexthdr udp accept) delivered dropped
60, with the PR (exthdr dst exists accept) dropped delivered

4. Restore

After restartVPC livepatch=true, the router's configure.py matched the build again. Rule 17 rendered meta l4proto udp again, and the destination-options UDP packet was delivered again.

Result: pass.

  • Protocol-number rules render as meta l4proto <n>, and extension-header numbers render as exthdr <name> exists. Everything loads on nft 1.0.6.
  • A UDP rule given as protocol 17 now matches UDP behind a destination options or hop-by-hop header, ingress and egress. The stock script drops those packets.
  • Extension-header rules match their header, and nothing else.

Not covered

  • Routing (43), fragment (44) and mobility (135) headers. Only the mapping for 0 and 60 was exercised on traffic. 43, 44 and 135 come from the same table in the code.
  • Redundant VPC routers, and templates older than 4.22.0.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants